-
Notifications
You must be signed in to change notification settings - Fork 537
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace useTanStack with useQuery #10345
Replace useTanStack with useQuery #10345
Conversation
WalkthroughThe changes refactor data fetching across several components by replacing the custom hook Changes
Sequence Diagram(s)sequenceDiagram
participant C as Component
participant Q as useQuery Hook
participant API as API/Query Function
participant UI as UI Renderer
C->>Q: Initialize query with queryKey & queryFn
Q->>API: Execute API call with structured parameters
API-->>Q: Return fetched data
Q-->>C: Provide data and loading state
C->>UI: Render updated information
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/components/Files/FileUpload.tsx (1)
137-147
: Potential optimization of pagination
The logic for offset-based pagination works, but using infinite queries or another approach could simplify caching and reduce repeated fetches.src/components/Common/UserAutocompleteFormField.tsx (1)
55-68
: Conditional disabling logic
The check for empty data upon an unfilled query is helpful, but consider grouping these conditions for readability or adding a short comment for clarity.src/components/Files/FileBlock.tsx (1)
1-1
: LGTM! Well-structured query implementation.The migration to
useQuery
follows React Query best practices with a well-organized query key structure. Consider adding error handling for failed queries.const { data: fileData } = useQuery({ queryKey: ["file", { id: file.id, type: fileManager.type, associating_id }], queryFn: query(routes.retrieveUpload, { queryParams: { file_type: fileManager.type, associating_id }, pathParams: { id: file.id || "" }, }), enabled: filetype === "AUDIO" && !file.is_archived, + retry: false, + onError: (error) => { + console.error("Failed to fetch file data:", error); + }, });Also applies to: 16-16, 37-43
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/CAREUI/misc/PaginatedList.tsx
(1 hunks)src/components/Common/UserAutocompleteFormField.tsx
(4 hunks)src/components/Files/FileBlock.tsx
(4 hunks)src/components/Files/FileUpload.tsx
(4 hunks)src/components/Patient/FileUploadPage.tsx
(2 hunks)src/components/Resource/ResourceDetailsUpdate.tsx
(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (19)
src/components/Files/FileUpload.tsx (9)
1-1
: Good integration of the react-query library
This import properly sets up the usage ofuseQuery
anduseQueryClient
.
23-23
: Consistent import naming
Using a descriptive import namequery
to call the underlying request utility is clear and intuitive.
93-93
: Using useQueryClient for cache invalidation
This will enable refetch and invalidation across queries for this component. Good approach!
123-135
: Refetching multiple queries
CallinginvalidateQueries
for active, archived, and discharge_summary is a clear, decoupled approach to ensure fresh data. Nicely done!
150-160
: Extend the approach used by activeFiles
The structure closely mirrors the active queries usage, ensuring consistency. Good job.
163-177
: Conditionally enabled query
Usingenabled: type === "consultation"
is an efficient way to fetch discharge summaries only when needed. Well done.
180-185
: Organized mapping of queries
Grouping queries by tab states keeps the code DRY and easy to manage. Nice approach.
188-188
: Single aggregated loading state
Combining all queries'isLoading
states might obscure partial loading conditions. Verify if partial loading states need to be distinguished.
194-194
: Conditional discharge summary tab
Automatically showing the tab only if results exist is user-friendly and reduces clutter.src/components/Patient/FileUploadPage.tsx (3)
1-1
: UseQuery import
ImportinguseQuery
from@tanstack/react-query
is consistent with the query integration approach across the codebase.
7-7
: Switching to new 'query' utility
Explicitly importing thequery
utility is consistent with the rest of the refactor using the same pattern.
17-22
: Conditional patient data fetching
Usingenabled: !!patientId
ensures queries are only fetched when thepatientId
is valid, preventing unnecessary requests.src/components/Common/UserAutocompleteFormField.tsx (5)
1-1
: Import from react-query
AdoptinguseQuery
aligns with the broader codebase transition to TanStack Query for state management.
14-14
: Importing the query utility
Using the samequery
utility fosters consistency across the application.
37-37
: Rename for clarity
Renaming fromquery
toqueryParam
clarifies its purpose as a variable holding the user's search text.
40-50
: useQuery for user list
ThequeryKey
andqueryFn
structure is well-defined, ensuring the user data is fetched with minimal overhead.
101-102
: Synchronize query param and loading
onQuery
updates thequeryParam
, whileisLoading
accurately reflects query status. Good approach to unify search with the new API.src/components/Files/FileBlock.tsx (1)
89-89
: LGTM! Safe data access with optional chaining.The use of optional chaining prevents potential runtime errors when accessing the URL.
src/components/Resource/ResourceDetailsUpdate.tsx (1)
1-1
: LGTM! Clean import updates.The imports are correctly updated to support the migration to React Query.
Also applies to: 4-4, 30-30
@rithviknishad @Jacobjeevan this PR is ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/components/Files/FileUpload.tsx (4)
124-136
: Consider optimizing query invalidation.While the current implementation works correctly, you could simplify it using partial key matching.
const refetchAll = () => { - queryClient.invalidateQueries({ - queryKey: ["viewUpload", "active", type, associatedId], - }); - queryClient.invalidateQueries({ - queryKey: ["viewUpload", "archived", type, associatedId], - }); - if (type === "consultation") { - queryClient.invalidateQueries({ - queryKey: ["viewUpload", "discharge_summary", associatedId], - }); - } + queryClient.invalidateQueries({ + queryKey: ["viewUpload"], + predicate: (query) => + query.queryKey[2] === type || + (type === "consultation" && query.queryKey[2] === "discharge_summary"), + }); };
138-148
: Add error handling for the active files query.Consider handling query errors to improve user experience.
const { data: activeFiles, isLoading: activeFilesLoading } = useQuery({ queryKey: ["viewUpload", "active", type, associatedId, offset], queryFn: query(routes.viewUpload, { queryParams: { file_type: type, associating_id: associatedId, is_archived: false, limit: RESULTS_PER_PAGE_LIMIT, offset: offset, }, }), + onError: (error) => { + // Handle error (e.g., show toast notification) + console.error("Failed to fetch active files:", error); + }, });
164-178
: Enhance type safety for discharge summary query.Consider adding a type guard for the consultation type check.
+const isConsultationType = (type: string): type is 'consultation' => type === 'consultation'; const { data: dischargeSummary, isLoading: dischargeSummaryLoading } = useQuery({ queryKey: ["viewUpload", "discharge_summary", associatedId, offset], queryFn: query(routes.viewUpload, { queryParams: { file_type: "discharge_summary", associating_id: associatedId, is_archived: false, limit: RESULTS_PER_PAGE_LIMIT, offset: offset, silent: true, }, }), - enabled: type === "consultation", + enabled: isConsultationType(type), });
181-186
: Consider memoizing the queries object.To prevent unnecessary re-renders, consider using useMemo for the queries object.
+const queries = useMemo(() => ({ -const queries = { UNARCHIVED: { data: activeFiles, isLoading: activeFilesLoading }, ARCHIVED: { data: archivedFiles, isLoading: archivedFilesLoading }, DISCHARGE_SUMMARY: { data: dischargeSummary, isLoading: dischargeSummaryLoading, }, -}; +}), [activeFiles, activeFilesLoading, archivedFiles, archivedFilesLoading, dischargeSummary, dischargeSummaryLoading]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Files/FileUpload.tsx
(4 hunks)
🧰 Additional context used
📓 Learnings (1)
📓 Common learnings
Learnt from: AdityaJ2305
PR: ohcnetwork/care_fe#10345
File: src/components/Resource/ResourceDetailsUpdate.tsx:132-145
Timestamp: 2025-01-31T22:13:06.153Z
Learning: The migration from useTanStackQueryInstead to useQuery in React components requires:
1. Updating types to use UseQueryResult instead of custom hook return type
2. Replacing loading with isLoading property
3. Using queryKey array for cache management
4. Using queryFn with query utility function
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (2)
src/components/Files/FileUpload.tsx (2)
1-1
: LGTM! Clean migration to @tanstack/react-query.The import changes and query client setup align with the migration requirements.
Also applies to: 24-24, 94-94
151-161
: Apply similar error handling to archived files query.Similar to the active files query, consider adding error handling here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/Patient/FileUploadPage.tsx (1)
17-23
: Consider adding type annotations for better type safety.The query configuration correctly follows the migration guidelines. However, we could enhance type safety.
Consider adding explicit type annotations:
- const { data: patient } = useQuery({ + const { data: patient } = useQuery<PatientType>({ queryKey: ["patient", patientId], queryFn: query(routes.getPatient, { pathParams: { id: patientId }, }), enabled: !!patientId, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Patient/FileUploadPage.tsx
(2 hunks)
🧰 Additional context used
📓 Learnings (1)
📓 Common learnings
Learnt from: AdityaJ2305
PR: ohcnetwork/care_fe#10345
File: src/components/Resource/ResourceDetailsUpdate.tsx:132-145
Timestamp: 2025-01-31T22:13:06.153Z
Learning: The migration from useTanStackQueryInstead to useQuery in React components requires:
1. Updating types to use UseQueryResult instead of custom hook return type
2. Replacing loading with isLoading property
3. Using queryKey array for cache management
4. Using queryFn with query utility function
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (2)
src/components/Patient/FileUploadPage.tsx (2)
1-1
: LGTM! Imports are correctly updated for React Query migration.The imports align with the migration requirements, bringing in both the
useQuery
hook and the query utility function.Also applies to: 7-7
18-18
: Query key is correctly structured.Addressing the previous review comment about
facilityId
: The query key correctly uses only["patient", patientId]
as it's fetching patient data. ThefacilityId
is appropriately used only for navigation and breadcrumbs, not as part of the cache key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/Patient/FileUploadPage.tsx (1)
18-24
: Consider destructuring isLoading if loading states are needed.The useQuery implementation follows best practices with proper query key structure and query function. However, if loading states are needed for UI feedback, consider destructuring isLoading from useQuery.
- const { data: patient } = useQuery<PatientModel>({ + const { data: patient, isLoading } = useQuery<PatientModel>({
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Patient/FileUploadPage.tsx
(2 hunks)
🧰 Additional context used
📓 Learnings (1)
📓 Common learnings
Learnt from: AdityaJ2305
PR: ohcnetwork/care_fe#10345
File: src/components/Resource/ResourceDetailsUpdate.tsx:132-145
Timestamp: 2025-01-31T22:13:06.153Z
Learning: The migration from useTanStackQueryInstead to useQuery in React components requires:
1. Updating types to use UseQueryResult instead of custom hook return type
2. Replacing loading with isLoading property
3. Using queryKey array for cache management
4. Using queryFn with query utility function
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Test
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
- GitHub Check: CodeQL-Build
🔇 Additional comments (2)
src/components/Patient/FileUploadPage.tsx (2)
Line range hint
1-16
: LGTM! Import statements and type declarations are well-structured.The imports correctly reflect the migration from useTanStackQueryInstead to useQuery, and the component props are properly typed.
Line range hint
26-46
: LGTM! Component rendering logic is clean and maintainable.The Page and FileUpload components are properly configured with appropriate props and conditional logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, a minor thing
LGTM |
@AdityaJ2305 Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌 |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
useQuery
hook.